fix(desktop): deliver mid-session tail append after loading history - #4066

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 1c60402fc231ab7ec9bf25d89590bbeb10027d06 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +76/-2). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

• Read gh pr diff 4066 (fix: desktop transcript replica, 2 files) and confirmed the new await #replaceWithRange(...) at desktop-transcript-replica.ts:365-378 and the helper at :267-296 with only #assertOpen() after await page.
• Checked exact-head CI: label SUCCESS, test run 33152715570 SUCCESS (affected tests, Runtime Host, Desktop E2E, Browser smoke, Storybook, CLI release candidate build all green), OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — NO-GO — 1×P2 (worst P2)

• P2 — Single submission-hygiene root cause — the PR body replaces the repository template (Problem/Root cause/Fix/Tests) and omits the required Summary/Verification/AI-use selection/Checklist/behavior fields. The body also states “Generated with Claude Code,” yet the only materially AI-authored commit carries only Co-Authored-By and lacks the Generated-by: Claude Code trailer required by CONTRIBUTING.md:34, violating CONTRIBUTING.md:81. Restore the template, fill the required selections, and amend the trailer.

No other Standards/code finding; the production increment reuses the existing #replaceWithRange seam with net entropy reduction and a green Fowler baseline.

Spec — NO-GO — 1×P2 (worst P2)

• P2 — The async re-anchor can resurrect a non-resident replica after discard — apps/desktop/src/main/desktop-transcript-replica.ts:365-378 does await #replaceWithRange(...) into helper :267-296, which after await page only checks #assertOpen() and does not re-check #resident. It then clears/installs durable range, advances the watermark, and publishes. Concrete race: let loadTranscriptPage stay pending; with hasNewer=true start advance(5); while pending call synchronous discard() (:331-341, which keeps the replica open but marks it non-resident); on page resolve the callback reloads and publishes, undoing eviction and the memory bound. The ordinary paged catch-up at :386-396 already correctly guards with if (!this.#resident) return. Add the same guard after the replacement await before any mutation and add a deferred-page regression: discard, then resolve, asserting no upsert/repopulation.

The happy path is otherwise correct: the new test creates hasNewer, advances to seq 5, receives a durable upsert, and the watermark advances; a higher concurrent target is safely converged via #targetThrough plus finally reschedule; tail anchor/direction, overlay completion, and oldest-edge eviction are correct.

Verification: git diff --check PASS, changed-file Biome PASS, ASF headers PASS, worktree clean, head unchanged. Focused tests were not rerun in the detached read-only worktree; 33152715570 is now SUCCESS with affected tests, Runtime Host, Desktop E2E, Browser smoke, Storybook, and CLI release candidate build all green — terminal CI is green.

What I did not judge: a full discard → page-resolve E2E with a real Host was not executed beyond the race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P2 (template/trailer) and Spec 1×P2 (re-anchor guard) remain; despite label and test both SUCCESS, head 1c60402f is still not merge-ready due to the two P2s. Seal: notes/pr-4066-provisional.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Thanks — both findings were real; addressed at c7401fed3.

Spec P2 (re-anchor can resurrect a discarded replica). Confirmed. Making the #hasNewer branch await #replaceWithRange(...) turned a previously synchronous branch async, so a concurrent discard() can flip #resident to false while the page is in flight, and the resolved callback would repopulate durable state past the memory bound. Fixed by re-checking #resident after the await inside #replaceWithRange, before any mutation/publish — the same guard the paged catch-up already uses. Added a deferred-page regression: it parks catch-up inside the re-anchor's page load, discard()s, then resolves the page and asserts no upsert and no repopulation (residentBytes stays 0). It fails without the guard (durableUpserts = [5], residentBytes grows back) and passes with it.

Standards P2 (submission hygiene). Restored the repository PR template (Summary / Verification / AI use selection / Checklist / behavior field) and amended the commit to carry the Generated-by: Claude Code trailer alongside Co-Authored-By, per CONTRIBUTING.md. Dropped a stray Refs #4024 (that is the unrelated font-size PR; this bug has no tracking issue).

Local re-verification on the amended head: build:main, typecheck, and biome check clean; desktop-transcript-range-store 17/17; runtime-host-session-observer 37/37.

Follow-up (af8694d2e): swept the rest of this file for the same "state can change across await" class the review flagged, and found one more latent instance.

#loadBefore awaited an older-history page and then installed it while only asserting #closed — so a discard() (memory reclaim for a non-visible session) landing during the load would be silently undone: the resolved page repopulated durable state and grew #residentBytes past the bound, exactly the resurrection the re-anchor guard now prevents. Added the same #resident re-check plus a deferred-page regression that discards mid-flight and asserts no publish and no repopulation (fails without the guard).

The remaining awaiting paths were reviewed and need no change:

• the paged catch-up already re-checks #resident before and after its await;
• the post-loop empty publish is unreachable once residency flips, because the expectedSequence watermark check trips first.

Transcript suite 18/18, observer 37/37, biome + typecheck clean.

This is a synthesis of the independent blind review by @Sol-404ARE at exact head af8694d2e8ee94785e3eb25fa605282d71059312 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +242/-2). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against 1c60402f.

What I checked myself:

• Read the incremental diff 1c60402f → af8694d2 (2 files +166, two rewritten commits c7401fed35 + af8694d2e8, guarding #loadBefore/#replaceWithRange/catch-up) and confirmed the new guards and the three deferred-page regressions.
• Checked exact-head CI: test run 33156432378 SUCCESS (affected tests, Runtime Host, Desktop E2E, Browser smoke, alignment audit, Storybook, CLI release candidate build all green), label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review (prior 1c60402f had Standards 1×P2 + Spec 1×P2):

Standards — NO-GO — 1×P2 + 1×P3 (worst P2)

• Closed: prior P2 — PR body now restores the template with the exact AI-use selection (Generated-by: Claude Code trailer now present on both rewritten commits with valid co-author trailers).
• New P2 — PR body is stale on the new exact head — the body still says 17/17, both new tests, two-line production change ... and both regression tests, singular the affected commit. The exact head now has 18 tests, 3 new regressions, 2 production guards ( #replaceWithRange + #loadBefore ), and 2 affected commits. Update the body to the exact 18-test counts and note whether that exact head was re-run.
• New P3 — judgment-only Duplicated Code — three neighboring regressions duplicate ~219 lines of the same five-message corpus, page/bootstrap, fake handle/decoder, history/change collector, and byte-budget plumbing. Extract a small history-window/deferred-page fixture. Production entropy decreases, test entropy increases.

Spec — NO-GO — 1×P2 (worst P2)

• Closed: prior P2 — #replaceWithRange and #loadBefore now have post-decode #resident guards that prevent late reload/publish after discard, each with a deterministic deferred-page regression — correctly closed.
• New P2 — production-reachable contiguous #catchUp discard — the ordinary contiguous catch-up at apps/desktop/src/main/desktop-transcript-replica.ts:389-399 can be discarded by another observed Session's global cache touch/LRU discard in runtime-host-session-observer.ts:1382-1415 while the page await is pending. On page resolve the callback at :400-402 returns, but expectedSequence is not advanced, so :420-423 throws correlation_changed; the subscription owner at :419-429 does not treat this as recoverable and the Session goes terminal. Fix: after #withDecodedPage, before the continuation/watermark check, add if (!this.#resident) return, and add a deferred contiguous-page discard regression. A prior provisional finding about coalesced advance was withdrawn because the sole external caller is the observer and live/pending frames are serially awaited.

Verification: full/incremental git diff --check PASS, both files Biome PASS, ASF header audit PASS, worktree clean, head unchanged. Focused tests were not rerun in the detached read-only worktree; test is now SUCCESS with all affected workspace, Runtime Host, Desktop E2E, Browser, alignment, Storybook, and CLI checks green — terminal CI is green.

What I did not judge: a true LRU-discard → page-resolve → correlation_changed → terminal-failure E2E with two observed Sessions was not executed beyond race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P2 (body staleness) + 1×P3 and Spec 1×P2 (catchUp guard) remain; despite label and test both SUCCESS, head af8694d2 is still not merge-ready due to the two P2+P3 findings. Seal: notes/pr-4066-af86-freshness-provisional.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Thanks for the third pass. Addressed the two P2s at 65c7e6942.

Spec P2 — production-reachable contiguous #catchUp discard. Confirmed, and you caught something I got wrong. In my previous follow-up I claimed the post-loop empty publish was "unreachable because the expectedSequence watermark check trips first" — I saw the check trip but mistook the throw for protection. It isn't: when another observed session's LRU discard() lands during a contiguous direction: 'newer' page await, the per-page callback returns early (no repopulation, good), but expectedSequence is left short of the watermark, so :401 throws correlation_changed and the subscription owner drives the session terminal — a benign memory reclaim turned fatal. Fixed by re-checking #resident after the page loop, before the watermark check, so catch-up returns cleanly and a later resume re-runs it. Added a deferred-page regression that discards while a contiguous catch-up page is in flight and asserts advance() resolves (not rejects) with no repopulation; it fails without the guard (advance() rejects with the watermark error).

That makes three #resident post-await guards now, one per awaiting path in this file (#replaceWithRange, #loadBefore, #catchUp), which I believe closes this class here.

Standards P2 — stale PR body. Fixed. Body now reflects the exact head: 19 tests, 3 production guards, 3 affected commits (all carrying the Generated-by trailer), and the four fail-without-guard regressions named individually. I also added a Verification note: my local workspace deps have drifted ahead of this branch's base, so the full-project build:main no longer typechecks some unrelated management/SSH test files; the changed files + their transitive imports compile cleanly in isolation and CI test is green at the exact head.

Standards P3 — test duplication (judgment). Acknowledged — the three discard-race regressions do repeat the five-message corpus, fake handle/decoder, and change collector. I deliberately kept each self-contained so its fixture documents the specific interleaving it exercises (tail re-anchor vs. older-history load vs. contiguous catch-up differ in bootstrap shape, budget, and which page is gated). Happy to extract a shared deferred-page fixture helper if you'd prefer the deduplication over the locality — your call, since it's a test-only readability tradeoff.

Verification on the new head 65c7e6942: desktop-transcript-range-store 19/19, biome clean on both changed files.

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 65c7e6942d7232ff174a825f9a89b18749530012 (base a956b1ae04aa7421a749931006a6df8fe564fc60, 2 files +329/-2). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against af8694d2.

What I checked myself:

• Read the incremental diff af8694d2 → 65c7e694 (1 commit +87, guard plus deferred contiguous-page regression :479-556) and confirmed the new post-loop #resident guard at desktop-transcript-replica.ts:421-428 sits before expectedSequence/correlation_changed (:429-430) and watermark/empty-publish (:432-433) with no await in between.
• Checked exact-head CI: test run 33162522534 SUCCESS, label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review (prior af8694d2 had Standards 1×P2+1×P3 / Spec 1×P2):

Standards — GO with non-blocking cleanup — 1×P3 (worst P3)

• Closed: prior stale-body P2 — the PR body now correctly states 19/19, four regressions, three post-await guards, and three affected commits; Generated-by: Claude Code trailers are present on all three commits with branch/title hygiene.
• Retained P3 — judgment-only Duplicated Code — desktop-transcript-range-store.test.ts:259-556 is now four neighboring large transcript/race fixtures; new :479-556 repeats the five-message setup, page/bootstrap/overlay, deferred gate, fixture handle, change collector, and discard/release + resident/byte assertions. Extracting a deferred-page/discard scenario builder would reduce this systemic duplication — still counted as one non-blocking P3, not a new gate.

Runtime failure-state entropy is reduced; test-maintenance entropy is slightly increased.

Spec — GO — 0 P0–P3

• Closed: prior production-reachable contiguous #catchUp P2 — discard now flips residency during the page load/decode; the callback at desktop-transcript-replica.ts:389-420 returns with no mutation, and the new post-loop guard at :421-428 returns before expectedSequence/correlation_changed and before watermark/empty-publish. Because no await exists between the guard and the synchronous checks, no new discard point exists, so the Session no longer goes terminal. Resume semantics are also correct: a discarded replica does not become resident; a later open refreshes the subscription and prepares a new replica, and advance()'s finalizer only bumps the nonresident watermark without repopulating.

• New regression :479-556 deterministically parks on a contiguous newer-page load, discards, resolves, and proves advance() does not reject, has no seq-5 upsert, remains nonresident, and residentBytes==0. It does not directly assert changes.length===0, so it cannot alone catch an empty publish, but the current guard placement makes that publish unreachable — therefore no finding.

Previously closed #replaceWithRange/#loadBefore seams remain closed; coalesced-advance withdrawal remains.

Verification: incremental git diff --check PASS, both-files Biome PASS, ASF headers PASS, exact test inventory 19 PASS, worktree clean, head unchanged. test SUCCESS; direct source-test in the detached worktree lacked workspace resolution and was excluded as an environment issue, not a product failure.

What I did not judge: a true LRU-discard during the contiguous page load with a second observed Session was not executed beyond the race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P3 (non-blocking) and Spec 0 — no P0–P2, test/label green, head 65c7e69 is ready to be approved and merged once a write holder approves (the remaining Duplicated-Code P3 is a cleanup suggestion). Seal: notes/pr-4066-65c7-freshness.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.